Conversation
crates/attestation/src/azure/mod.rs
Outdated
| pub async fn detect_azure_cvm() -> Result<bool, reqwest::Error> { | ||
| let client = reqwest::Client::builder().no_proxy().timeout(Duration::from_secs(2)).build()?; | ||
|
|
||
| let resp = client.get(AZURE_METADATA_API).header("Metadata", "true").send().await; | ||
|
|
||
| if let Ok(r) = resp && | ||
| r.status().is_success() && | ||
| r.headers() | ||
| .get(reqwest::header::CONTENT_TYPE) | ||
| .and_then(|value| value.to_str().ok()) | ||
| .is_some_and(|value| value.starts_with("application/json")) | ||
| { | ||
| return Ok(az_tdx_vtpm::is_tdx_cvm().unwrap_or(false)); | ||
| } | ||
| Ok(false) | ||
| } |
There was a problem hiding this comment.
question:
there's quite a bunch of stuff happening here, at any point of which things could go off the happy-path.
should we perhaps log that? even if at debug/trace level?
(for the operator it could be important to be able to tell if the check has "decided" not-azure b/c of timeout, or b/c of missing header, or whatever else).
alternatively, this could be enriched Error type instead of the one from reqwest.
There was a problem hiding this comment.
I agree. Probably it should only return false if we appear to be not on azure because the metadata api is not available. Unusual cases like that the metadata API is available but no response header should be treated as an error with an explicit type.
There was a problem hiding this comment.
@0x416e746f6e I have added logging, and slightly improved error handling.
Closes #11
This aims to improve detection that we are on a TDX Azure instance.
Previously we attempted to do an Azure vTPM attestation and if it succeeds, assume we are on Azure.
Now we first hit the Azure metadata API, and if that succeeds, we call
az_tdx_vtpm::is_tdx_cvm()](https://docs.rs/az-tdx-vtpm/latest/az_tdx_vtpm/fn.is_tdx_cvm.html) which internally gets an HCL report from the vTPM and checks that the report type is TDX.The advantage of first hitting the metadata API is that we do not get errors logged when this fails on non-azure platforms.
Im not sure if there is an advantage of using
is_tdx_cvm()rather than doing a full attestation. It makes the check faster, but maybe does not fully guarantee that attestation will succeed.I have not yet tested this on Azure.